Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add newer class for purging policies #103

Closed
wants to merge 1 commit into from

Conversation

craigcomstock
Copy link
Contributor

Running on 3.24.x I get the following report/warning

R: `cfengine_internal_purge_policies` no longer has any effect. Please use `cfengine_internal_purge_policies_disabled` instead, to choose where you want to disable purging or remove the class completely if you want purging enabled everywhere (the new default in 3.18+).

I figured we should add both? So that this demo module works on older versions but the note mentions 3.18+ so maybe we can just remove instead).

Running on 3.24.x I get the following report/warning

```
R: `cfengine_internal_purge_policies` no longer has any effect. Please use `cfengine_internal_purge_policies_disabled` instead, to choose where you want to disable purging or remove the class completely if you want purging enabled everywhere (the new default in 3.18+).
```

I figured we should add both? So that this demo module works on older versions but the note mentions 3.18+ so maybe we can just remove instead).
Copy link
Member

@nickanderson nickanderson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should simply delete both.

Comment on lines 4 to +5
"cfengine_internal_purge_policies": ["any"],
"cfengine_internal_purge_policies_disabled": ["any"],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"cfengine_internal_purge_policies": ["any"],
"cfengine_internal_purge_policies_disabled": ["any"],

The class cfengine_internal_purge_policies would enable that feature on older versions of the MPF. When this feature was enabled files inside of /var/cfengine/inputs that did not exist on the servers /var/cfengine/masterfiles would be removed from the client, in effect a sync behavior instead of the default to just copy files on top if changes were made, leaving extra files alone.

That default changed and so now that behavior is the default. If you previously had cfengine_internal_purge_policies defined then today you should simply disable the definition of that class to keep that sync behavior. Alternatively, if you desire the old behavior where extra files were not cleaned up then you would instead define cfengine_internal_purge_policies_disabled.

Since, this was enabling that feature which is now the default, I think that both of these should just be removed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After thinking about this for a moment another option would be to adjust cfengine_internal_purge_policies to target cfengine_3_18 if the desire is to keep that behavior for older clients that may still be using this. Otherwise, still best to remove both i think.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agreed. I didn't read things quite right. Will adjust.

Comment on lines 4 to +5
"cfengine_internal_purge_policies": ["any"],
"cfengine_internal_purge_policies_disabled": ["any"],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After thinking about this for a moment another option would be to adjust cfengine_internal_purge_policies to target cfengine_3_18 if the desire is to keep that behavior for older clients that may still be using this. Otherwise, still best to remove both i think.

@craigcomstock
Copy link
Contributor Author

closed in preference for more appropriate change: #105

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants